Skip to content

fix(windows-miner): back off transient header failures - #8018

Merged
Scottcjn merged 3 commits into
Scottcjn:mainfrom
guoqiangliu-ocean:agent/windows-header-retry-policy
Jul 21, 2026
Merged

fix(windows-miner): back off transient header failures#8018
Scottcjn merged 3 commits into
Scottcjn:mainfrom
guoqiangliu-ocean:agent/windows-header-retry-policy

Conversation

@guoqiangliu-ocean

@guoqiangliu-ocean guoqiangliu-ocean commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Closes #7368

Bounty: Scottcjn/rustchain-bounties#16252
BCOS: BCOS-L1

What this completes

#7565 stopped the original hot loop and surfaced the rejection diagnostic, but it marked every failed slot handled. A temporary connection failure or restarting node therefore never retried during the slot. This PR completes the missing retry-policy part of #7368:

  • terminal client/auth responses are marked handled and never resubmitted;
  • connection failures plus HTTP 408/425/429/5xx responses use 10, 20, 40, 80, 160, then capped 300 second backoff;
  • each retry returns through the mining loop, which rebuilds and signs a fresh header;
  • headless output says class=terminal no_retry or class=retryable retry_in=Ns and retains the node's diagnostic;
  • bootstrap SHA-256 pins are updated with the miner artifact.

Before / after reproduction

Before this PR on current main, a network blip was treated as permanently handled:

12:00:00 [share] slot=27456 submitted=1 accepted=0 FAIL error=header request failed: connection refused
12:00:10 slot=27456 suppressed by _last_submitted_slot (no retry after recovery)
12:00:20 slot=27456 suppressed by _last_submitted_slot (no retry after recovery)

After this PR, temporary failures rebuild the header and back off:

12:00:00 [share] slot=27456 submitted=1 accepted=0 FAIL error=HTTP 503 error=node restarting class=retryable retry_in=10s
12:00:10 [share] slot=27456 submitted=2 accepted=0 FAIL error=HTTP 503 error=node restarting class=retryable retry_in=20s
12:00:20 retry gate closed (next attempt remains at 12:00:30)

Terminal rejection remains one-shot. This is the actual output from a one-time, identity-free invalid-header probe against the repository's public node endpoint on 2026-07-20:

[share] slot=-7368 submitted=1 accepted=0 FAIL error=HTTP 400 error=missing fields class=terminal no_retry

The same run recorded last_submitted_slot=-7368, retry_slot=null; _header_submission_due(-7368) stays false at later timestamps, so there is no second POST of that rejected header.

Verification

34 passed in 13.00s

Covered suites:

  • Windows header retry regression tests (terminal JSON/plain-text responses, connection failure, 503, exponential cap, log formatting)
  • Windows bootstrap checksum, Python 3.9 annotation, chain identity, and headless lifecycle tests
  • attestation signing and setup-miner download tests
  • python miners/windows/check_bootstrap_hashes.py -> Windows bootstrap hashes are in sync.
  • python -m py_compile miners/windows/rustchain_windows_miner.py

The live probe bypassed miner initialization and sent no wallet, key, signature, or user identity; it only exercised the public endpoint's terminal validation response.

@github-actions

github-actions Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

  • Non-doc PRs have a BCOS-L1 or BCOS-L2 label
  • Doc-only PRs are exempt from BCOS tier labels when they only touch docs/**, *.md, or common image/PDF files
  • New code files include an SPDX license header
  • You've tested your changes against the live node

Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150)

A maintainer will review your PR soon. Thanks for contributing!

@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) tests Test suite changes size/M PR: 51-200 lines labels Jul 19, 2026
@guoqiangliu-ocean guoqiangliu-ocean mentioned this pull request Jul 19, 2026
Open
4 tasks
elevasyncsolutions-jpg
elevasyncsolutions-jpg reviewed Jul 20, 2026
View reviewed changes

@elevasyncsolutions-jpg elevasyncsolutions-jpg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of PR #8018 — fix(windows-miner): back off transient header failures

Overall this is a well-structured fix for #7368. Two substantive observations:

  1. Missing jitter in exponential backoff — The _schedule_header_retry uses pure exponential backoff (10, 20, 40, 80…). If multiple miners lose connectivity at the same time (e.g. a node restart), they will retry in lockstep. Adding a small random jitter (±25% of the delay) would spread retries and reduce node load on recovery. See AWS retry best practices.

  2. Redundant isinstance check — In _is_retryable_header_status, the check isinstance(status_code, int) and 500 <= status_code <= 599 has an unnecessary isinstance guard — requests always returns int for status_code. The real guard is against the case where response is None (network error), but that path already sets last_header_retryable = True in the exception handler. The 5xx range check alone would be cleaner.

Positive: The separation of terminal vs retryable failures with the _reset_header_retry() mechanism is a significant improvement over the previous binary handled/not-handled logic. The bounded cap at 300s prevents runaway waits. Good defensive design.

@elevasyncsolutions-jpg elevasyncsolutions-jpg mentioned this pull request Jul 20, 2026
Open
@github-actions github-actions Bot added size/L PR: 201-500 lines and removed size/M PR: 51-200 lines labels Jul 21, 2026
@guoqiangliu-ocean

guoqiangliu-ocean commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the review feedback in 38b2890: retry jitter is now ±25% (while preserving the 300 s cap), and the redundant status-code type guard was removed. Bootstrap hashes were refreshed in abb066f. All 12 upstream checks, including the full 3,640-test CI job and Windows build, are green. Ready for maintainer review.

@Scottcjn
Scottcjn merged commit d684e61 into Scottcjn:main Jul 21, 2026
12 checks passed
@Scottcjn

Scottcjn commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Merged — bounded exponential backoff (10s base, 300s cap, retryable on 408/425/429) with the diagnostic surfaced, exactly what #16252 asked for. Awarded 12 RTC, sent to your wallet with the other 65. Thanks for the consistently clean work. — Sophia

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

RTC Reward

This merged PR earned 5 RTC — sent to guoqiangliu-ocean.

RustChain Bounty Program

JHON12091986
JHON12091986 reviewed Jul 21, 2026
View reviewed changes

@JHON12091986 JHON12091986 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: Looks good! Minor suggestion: ...

FlintLeng
FlintLeng reviewed Jul 22, 2026
View reviewed changes

@FlintLeng FlintLeng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #8018 审查报告

标题: fix(windows-miner): back off transient header failures
作者: guoqiangliu-ocean
改动: +202 / -23 行，4 个文件
SHA: abb066f4bf212de85931117765efaaa399e9818c

改动摘要

文件 改动
miners/windows/rustchain_windows_miner.py +100 / -17
tests/test_windows_headless_rejected_slot_retry.py +100 / -4
miners/windows/rustchain_miner_setup.bat +1 / -1
setup_miner.py +1 / -1

技术评估

问题背景

PR #7565 修复了 header rejection 的热循环问题，但将所有失败 slot 标记为已处理，导致临时性网络故障无法重试。

核心实现

  1. 终端响应 vs 可重试响应分类：

    • 终端响应（客户端认证错误、无效 header）→ 标记 handled，永不重试
    • 可重试响应（连接失败、HTTP 408/425/429/5xx）→ 指数退避重试

  2. 退避策略：

    • 退避序列：10s → 20s → 40s → 80s → 160s → 封顶 300s
    • 每次重试返回 mining loop，重新构建并签名 header

  3. 日志输出改进：

    • class=terminal no_retry — 终端失败，无重试
    • class=retryable retry_in=Ns — 可重试，显示下次尝试时间

测试覆盖

  • 新增 100 行测试代码覆盖：
    • 终端 JSON/plain-text 响应
    • 连接失败、HTTP 503
    • 指数退避上限验证
    • 日志格式验证
  • 34 个测试全部通过
  • Bootstrap SHA-256 校验通过
  • 实际向公共节点发送了无身份 probe 验证终端响应

安全性

  • 无敏感数据泄露
  • 重试策略合理，不会导致无限循环
  • 日志不包含钱包地址或签名

审查结论

✅ APPROVE

这是一个高质量的 bug 修复 PR：

  • 问题定位准确，修复逻辑清晰
  • 终端 vs 可重试的区分设计合理
  • 退避策略符合业界最佳实践
  • 测试覆盖充分，包含真实端点验证
  • 文档详尽，提供了 before/after 对比

Bounty 认领

钱包地址: RTC019e78d600fb3131c29d7ba80aba8fe644be426e
预估奖励: 2 RTC

Closes #7368 | Bounty: Scottcjn/rustchain-bounties#16252

@sophiaeagent-beep sophiaeagent-beep mentioned this pull request Jul 26, 2026
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers
@FlintLeng FlintLeng FlintLeng left review comments
@elevasyncsolutions-jpg elevasyncsolutions-jpg elevasyncsolutions-jpg left review comments
@JHON12091986 JHON12091986 JHON12091986 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) size/L PR: 201-500 lines tests Test suite changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Windows headless miner retries a rejected header every poll and hides the server diagnostic

5 participants

@guoqiangliu-ocean @Scottcjn @FlintLeng @elevasyncsolutions-jpg @JHON12091986